Add German translations #868

hbruch · 2022-10-17T15:27:12Z

Checklist

Appropriate branch selected (all PRs must first be merged to dev before they can be merged to master)
Any modified or new methods or classes have helpful JSDoc and code is thoroughly commented
The description lists all applicable issues this PR seeks to resolve
The description lists any configuration setting(s) that differ from the default settings
[x/?] All tests and CI builds passing

Description

This PR adds German translations and adds locale-dependent message retrieval to a number of components. Though not complete, I'd like to request a merge to avoid future merge conflicts, as a number of components where touched.

Other tasks still to done to have datatools fully internationalized:

Some components (e.g. the alerts module (lib/alerts/**) are not yet fully internationalized
A couple of messages (i.e. job names and status) are generated server-side and are not internationalized yet.
Time and date formatting performed by the moment javascript lib are not locale aware
Some messages from the react-bootstrap table component, e.g. Showing row x-y of z are not locale aware
GTFS-related field names and descriptions defined in GTFS.yml (see I18N issues #864)

Regarding the German translation, I decided to keep some terms in English, as I think they are commonly (or in GTFS context) used and understandable (e.g. deployment, trip, feed,...)

Finally, this PR includes a small dev doc regarding localization, to document some of my learnings.

Signed-off-by: Holger Bruch <[email protected]>

lib/admin/components/ServerSettings.js

-                <a href={serverDocUrl} target='_blank'>
-                Instructions for setting up OTP deployment servers on AWS
-                </a>
+                <a href={serverDocUrl} target='_blank'>{this.messages('awsSetup')}</a>


lib/manager/components/ProjectViewer.js

-            <a href={publicFeedsLink} target='_blank'>here</a>.
+            <strong>{this.messages('note')}</strong>
+            {this.messages('publicViewed')}
+            <a href={publicFeedsLink} target='_blank'>{this.messages('here')}</a>.


miles-grant-ibigroup

This is incredible! Thanks so much for your contribution and doing all this work so cleanly. Thanks for adding documentation as well.

I agree with you on doing the work in multiple PRs. As for the Denglish I understand your reasoning, although what do you think about "Schattenkopie" for snapshot?

Finally, I will look into why the e2e tests are failing. I suspect it has to do with this being an external PR.

miles-grant-ibigroup · 2022-10-18T18:42:27Z

i18n/german.yml

+  DeploymentsPanel:
+    autoDeploy:
+      deployWithErrors:
+        checklabel: Deployen, auch falls einige Feeds kritische Fehler aufweisen?


Hier ist nichts mit Haarwaschmittel... Aber da diese Arbeit oft in Englisch stattfindet ist das wahrscheinlich kein Problem.

miles-grant-ibigroup · 2022-10-18T18:43:07Z

i18n/german.yml

+    latestVersion: Aktuellste Version
+    status: Status
+  FeedSourceTableRow:
+    dateFormat: DD.MM.YYYY


miles-grant-ibigroup · 2022-10-18T18:52:05Z

lib/manager/components/FeedSourceTableRow.js

+  getVersionDisplayData (
+    validationSummary: ?ValidationSummary
+  ) {


This doesn't quite match previous component styles but I think we're refactoring this soon anyway so happy to leave it! Maybe we can add a comment

miles-grant-ibigroup · 2022-10-18T19:31:34Z

Yep the e2e tests are failing because the secrets are missing. They pass locally, so we can override them for this PR? Will leave it up to the second reviewer to decide

Signed-off-by: Holger Bruch <[email protected]>

derhuerst · 2022-10-19T14:33:51Z

[…] what do you think about "Schattenkopie" for snapshot?

From my intuition, people rather understand what "Schnappschuss" means than "Schattenkopie"; "Abbild", "Momentaufnahme" and "Zustand" also come to my mind.

derhuerst · 2022-10-19T14:43:17Z

From my experience on internationalised apps, i prefer having very specific translation IDs/keys.

Often,** when too short/broad translation keys are used, they get re-used in other places of the code base**, even though those places use them in a different context.

For example, while both a) the heading of the panel for uploading files to a cloud storage as well as b) the instruction on how to do it could use the translation ID cloud-upload, the two translations will likely require different lengths/verbosity, grammatical forms and tones; With two separate, more specific IDs cloud-upload-heading and cloud-upload-introductory-text, this confusion doesn't happen.

It seems much easier to, e.g. when you want to adapt all occurences of a certain word, just use the text search to adapt related but separate translations than to refactor places in the code that use the same translation ID even though they shouldn't.

Some projects even go so far as using the en-US variant as the translation ID, so that the context of the translations cannot be misinterpreted when adding another translation.

binh-dam-ibigroup · 2022-10-19T15:57:16Z

From my experience on internationalised apps, i prefer having very specific translation IDs/keys.

Often,** when too short/broad translation keys are used, they get re-used in other places of the code base**, even though those places use them in a different context.

The good thing with how datatools organizes strings is that all strings are tied to a component (see getComponentMessages(), which provides the context based on what that component displays while allowing the use of shorter ids.

binh-dam-ibigroup

Thanks so much for doing the heavy lifting, it looks good so far! Please see my suggestions for small tweaks to the documentation and some parts of the code.

binh-dam-ibigroup · 2022-10-19T14:03:32Z

docs/dev/localization.md

+	* import getComponentMessages from common/util.config
+	* assign getComponentMessages('<ComponentName>') to a component properties `messages`
+	* extract not yet translated messages from component files to `i18n/english.yml` and replace their original text by this.messages('<key>')
+	* in case the original message was haad some dynamic parts, you should create placeholders (using `%placeholder%` and use replace to substitute that string with the intended value). Note that numic parameters should be converted to strings.


Suggested change

* in case the original message was haad some dynamic parts, you should create placeholders (using `%placeholder%` and use replace to substitute that string with the intended value). Note that numic parameters should be converted to strings.

* in case the original message contains dynamic parts, you should create placeholders (e.g. `%placeholder%`, and replacing that string with the intended value after calling `this.messages('<key>')`). Note that numeric parameters should be converted to strings.

binh-dam-ibigroup · 2022-10-19T14:05:10Z

docs/dev/localization.md

+5. Add a new line to the CHANGELOG, e.g. `Add support for <new language>`
+5. Before commiting, run `yarn run lint-messages` or `yarn run test`


Add periods . at the end of sentences.

binh-dam-ibigroup · 2022-10-19T14:06:07Z

docs/dev/localization.md

+Note: console log messages are not intended to be localized
+
+### Open issues
+There are a few sections, which are not yet prepared for i18n and will not adapt to the user's locale:
+* The alerts module (`lib/alerts/**`) is not yet fully internationalized
+* A couple of messages (i.e. job names and status) are generated server-side and are not internationalized yet.
+* Time and date formatting performed by the moment javascript lib are not locale aware
+* Some messages from the react-bootstrap table component, e.g. `Showing row x-y of z`.
+* GTFS-related field names and descriptions defined in GTFS.yml


Add period at the end of sentences.

binh-dam-ibigroup · 2022-10-19T15:13:11Z

lib/common/components/InfoModal.js

@@ -15,6 +17,8 @@ type State = {
 }

 export default class InfoModal extends React.Component<Props, State> {
+  messages = getComponentMessages('InfoModal')


Revert changes to this file (there is no use of this.messages).

binh-dam-ibigroup · 2022-10-19T15:35:29Z

lib/manager/components/ProjectViewer.js

-            <strong>Note:</strong> Public feeds page can be viewed{' '}
-            <a href={publicFeedsLink} target='_blank'>here</a>.
+            <strong>{this.messages('note')}</strong>
+            {this.messages('publicViewed')}


Keep the {' '} after the <strong> tag.

I re-inserted the {' '} but had to insert an indented new line to ActiveProjectViewer.js.snap to get the test passing again. What exactly is the purpose of these explicitly rendered whitespaces?

At least in English, you want to have a blank space after a colon, such as in Note: Public feeds can be viewed.... Without the {' '}, React will collapse the white space from line returns and result is more difficult to read, as in Note:Public feeds.....

binh-dam-ibigroup · 2022-10-19T15:45:24Z

lib/manager/reducers/status.js

    case 'REQUEST_RTD_ALERTS':
-      return {...state, message: 'Loading alerts...'}
+      return {...state, message: messages(`${action.type}`)}


Suggested change

return {...state, message: messages(`${action.type}`)}

return {...state, message: messages(action.type)}

binh-dam-ibigroup · 2022-10-19T17:18:42Z

Because this pull request comes from a fork of datatools-ui, E2E tests cannot be run (GitHub does not pass secrets to PRs from forks), but I am inclined to merge (with changes indicated) if @miles-grant-ibigroup agrees too.

hbruch · 2022-10-20T20:42:39Z

Thanks for your thorough review, @binh-dam-ibigroup. All comments should be resolved, though I had to adapt ActiveProjectViewer.js.snap after re-inserting {' '} (see above).

hbruch added 3 commits October 17, 2022 16:58

Add German translations and extract some more messages from components

68b2013

Signed-off-by: Holger Bruch <[email protected]>

Add localization how-to docs

4ed249b

Signed-off-by: Holger Bruch <[email protected]>

Add new messages to polish.yml

8473ada

Signed-off-by: Holger Bruch <[email protected]>

miles-grant-ibigroup self-assigned this Oct 17, 2022

github-advanced-security bot found potential problems Oct 18, 2022

View reviewed changes

miles-grant-ibigroup approved these changes Oct 18, 2022

View reviewed changes

miles-grant-ibigroup assigned binh-dam-ibigroup and unassigned miles-grant-ibigroup Oct 18, 2022

hbruch added 2 commits October 19, 2022 09:23

Fixes bracket issue

5e1d7cb

Signed-off-by: Holger Bruch <[email protected]>

Fixes message key/translation issue

e03c12a

Signed-off-by: Holger Bruch <[email protected]>

binh-dam-ibigroup approved these changes Oct 19, 2022

View reviewed changes

Incorporate review feedback

bc36011

binh-dam-ibigroup merged commit 4828800 into ibi-group:dev Oct 24, 2022

miles-grant-ibigroup mentioned this pull request Nov 16, 2022

Add Missing Fetch Feed Message #876

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add German translations #868

Add German translations #868

hbruch commented Oct 17, 2022

miles-grant-ibigroup left a comment

miles-grant-ibigroup Oct 18, 2022

miles-grant-ibigroup Oct 18, 2022

miles-grant-ibigroup Oct 18, 2022

miles-grant-ibigroup commented Oct 18, 2022

derhuerst commented Oct 19, 2022

derhuerst commented Oct 19, 2022 •

edited

Loading

binh-dam-ibigroup commented Oct 19, 2022

binh-dam-ibigroup left a comment

binh-dam-ibigroup Oct 19, 2022

binh-dam-ibigroup Oct 19, 2022

binh-dam-ibigroup Oct 19, 2022

binh-dam-ibigroup Oct 19, 2022

binh-dam-ibigroup Oct 19, 2022

hbruch Oct 20, 2022

binh-dam-ibigroup Oct 20, 2022

binh-dam-ibigroup Oct 19, 2022

binh-dam-ibigroup commented Oct 19, 2022

hbruch commented Oct 20, 2022

	* in case the original message was haad some dynamic parts, you should create placeholders (using `%placeholder%` and use replace to substitute that string with the intended value). Note that numic parameters should be converted to strings.
	* in case the original message contains dynamic parts, you should create placeholders (e.g. `%placeholder%`, and replacing that string with the intended value after calling `this.messages('<key>')`). Note that numeric parameters should be converted to strings.

		5. Add a new line to the CHANGELOG, e.g. `Add support for <new language>`
		5. Before commiting, run `yarn run lint-messages` or `yarn run test`

	return {...state, message: messages(`${action.type}`)}
	return {...state, message: messages(action.type)}

Add German translations #868

Add German translations #868

Conversation

hbruch commented Oct 17, 2022

Checklist

Description

miles-grant-ibigroup left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miles-grant-ibigroup commented Oct 18, 2022

derhuerst commented Oct 19, 2022

derhuerst commented Oct 19, 2022 • edited Loading

binh-dam-ibigroup commented Oct 19, 2022

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

binh-dam-ibigroup commented Oct 19, 2022

hbruch commented Oct 20, 2022

derhuerst commented Oct 19, 2022 •

edited

Loading